Skip to content

refactor(dashnote): tidy auth UX + add delete confirmation modal#82

Merged
thephez merged 3 commits into
mainfrom
chore/login-cleanup
May 14, 2026
Merged

refactor(dashnote): tidy auth UX + add delete confirmation modal#82
thephez merged 3 commits into
mainfrom
chore/login-cleanup

Conversation

@thephez
Copy link
Copy Markdown
Collaborator

@thephez thephez commented May 13, 2026

Summary

  • Drop LoginModal duplicates of Settings controls. The contract-ID editor and "Forget this device" button now live only in the Settings panel; the modal's Advanced disclosure also hides for WIF input since identity-index is the only remaining knob there.
  • Unify auth CTAs under "Sign in". Renames Login/Log in across the IdentityCard, sidebar NavButton, LoginModal, and NotesWorkspace empty state. Hides the sidebar Sign in entry in authenticated/browsing (IdentityCard menu handles those). Adds a primary Sign in button to the Settings unauth empty state matching the Notes treatment. The e2e loginViaModal helper now routes through the IdentityCard menu when the session boots into browsing (rememberMe reload) since the sidebar entry is absent there.
  • Add note delete confirmation modal. Replaces window.confirm with a DeleteNoteModal that shows the note title in body copy, uses a danger-styled Delete button, disables both actions while a delete is in flight, and ignores Escape, Cancel, and backdrop dismiss mid-delete to keep parent state in sync.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Added confirmation modal when deleting notes for improved safety
  • Improvements

    • Standardized "Sign in" terminology throughout the app
    • Refined login flow and settings panel integration
    • Enhanced navigation button visibility based on authentication status

Review Change Stack

thephez and others added 3 commits May 13, 2026 17:42
The contract-ID editor and "Forget this device" button now live only in
the Settings panel; the LoginModal's Advanced disclosure also hides for
WIF input since identity-index is the only remaining knob.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renames Login/Log in to Sign in across the IdentityCard, sidebar
NavButton, LoginModal, and NotesWorkspace empty state. Hides the
sidebar Sign in entry in authenticated and browsing modes (the
IdentityCard menu handles those). Adds a primary Sign in button to
the SettingsPanel unauth empty state matching the NotesWorkspace
treatment. The e2e loginViaModal helper now routes through the
IdentityCard menu when the session boots into browsing (rememberMe
reload) since the sidebar entry is absent there.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces window.confirm with a DeleteNoteModal that shows the note
title in body copy, uses a danger-styled Delete button, disables both
actions while a delete is in flight, and ignores Escape, Cancel, and
backdrop dismiss mid-delete to keep parent state in sync.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

📝 Walkthrough

Walkthrough

Dashnote's authentication and deletion flows are refactored: a new DeleteNoteModal component replaces inline deletion with modal-driven confirmation, LoginModal removes contract-ID reuse and "Forget this device" features, UI labels change from "Login" to "Sign in", and SettingsPanel integrates a callback to trigger the login modal.

Changes

Auth & Delete Flow Redesign

Layer / File(s) Summary
Delete confirmation modal and workflow
src/components/DeleteNoteModal.tsx, src/components/NotesWorkspace.tsx, test/DeleteNoteModal.test.tsx, test/NotesWorkspace.test.tsx
DeleteNoteModal is introduced with props for title, delete state, and callbacks. NotesWorkspace refactors deletion into a two-step flow: requestDelete() opens the modal, confirmDelete() executes the async delete after confirmation. Tests verify modal rendering, callback invocation, and button disable states during deletion.
LoginModal contract simplification
src/components/LoginModal.tsx, test/LoginModal.test.tsx
Contract-ID state and syncing are removed; "Forget this device" action is deleted; remembered-identity panel only shows when an identity exists and the user is not switching; advanced settings UI is hidden when WIF input is detected. Modal title and submit button label update to "Sign in". Tests cover all label and behavior changes, including removal of the forgotten-identity test case.
Auth UI labels and component integration
src/components/AppShell.tsx, src/components/IdentityCard.tsx, src/components/SettingsPanel.tsx, src/App.tsx, test/IdentityCard.test.tsx, test/SettingsPanel.test.tsx
AppShell shows the sign-in nav button only in unauthenticated states (not "authenticated" or "browsing"). IdentityCard conditionally renders "Sign in" (disconnected) or "Switch identity" (authenticated). SettingsPanel accepts an onOpenLogin callback and wires its sign-in button to invoke it. App passes the callback to SettingsPanel. Tests verify all label changes and callback wiring.
E2E test suite and fixture updates
test/e2e/auth.spec.ts, test/e2e/fixtures.ts, test/e2e/smoke.spec.ts
E2E auth test replaces "forget device" coverage with identity-switch validation. Fixture helpers loginViaModal and deleteNoteByTitle are refactored to detect session state and use the correct UI entry point; delete confirmation now clicks a modal dialog button instead of auto-accepting a system prompt. Smoke tests update button label selectors and verify advanced-settings visibility behavior for mnemonic vs. WIF input.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • dashpay/platform-tutorials#80: Directly precedes this PR's "settings tab + focused login modal" flow wiring by establishing the callback prop contract between App and SettingsPanel.
  • dashpay/platform-tutorials#81: Introduces the Dashnote e2e suite, fixtures, and CI workflow that this PR's updated test helpers and E2E cases depend on.

Poem

🐰 A note shall delete with a modal's sweet say,
"Confirm, then we cleanse," no more rash away!
Sign in, not login—the button's renamed,
And contract ID reuse is gently unclaimed.
Settings now call when the Sign-in light gleams!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: tidying auth UX (standardizing to 'Sign in', removing duplicates, etc.) and adding a delete confirmation modal.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/login-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@example-apps/dashnote/test/e2e/fixtures.ts`:
- Around line 131-141: The branch should stop depending on the visible "Browsing
(read-only)" text and instead detect the actual sign-in affordance; change the
logic in the block containing openIdentityMenu, navButton and getByText to first
probe for a sign-in control (e.g., try page.getByRole("menuitem", { name: /^sign
in$/i }).isVisible() or a navButton match for /sign in$/i) and use whichever
affordance is present to click it; keep using openIdentityMenu only when the
identity menu is reachable (detectable) and fall back to navButton when a
top-level sign-in button exists, rather than branching on getByText("Browsing
(read-only)").
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 341efc28-9c6f-4a9c-abea-1843419c007e

📥 Commits

Reviewing files that changed from the base of the PR and between f600894 and c6a6d79.

⛔ Files ignored due to path filters (1)
  • example-apps/dashnote/public/favicon.svg is excluded by !**/*.svg
📒 Files selected for processing (15)
  • example-apps/dashnote/src/App.tsx
  • example-apps/dashnote/src/components/AppShell.tsx
  • example-apps/dashnote/src/components/DeleteNoteModal.tsx
  • example-apps/dashnote/src/components/IdentityCard.tsx
  • example-apps/dashnote/src/components/LoginModal.tsx
  • example-apps/dashnote/src/components/NotesWorkspace.tsx
  • example-apps/dashnote/src/components/SettingsPanel.tsx
  • example-apps/dashnote/test/DeleteNoteModal.test.tsx
  • example-apps/dashnote/test/IdentityCard.test.tsx
  • example-apps/dashnote/test/LoginModal.test.tsx
  • example-apps/dashnote/test/NotesWorkspace.test.tsx
  • example-apps/dashnote/test/SettingsPanel.test.tsx
  • example-apps/dashnote/test/e2e/auth.spec.ts
  • example-apps/dashnote/test/e2e/fixtures.ts
  • example-apps/dashnote/test/e2e/smoke.spec.ts

Comment thread example-apps/dashnote/test/e2e/fixtures.ts
@thephez thephez merged commit 474edb3 into main May 14, 2026
5 checks passed
@thephez thephez deleted the chore/login-cleanup branch May 14, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant